feat: implement Merchant model (Closes #39) - #45
Conversation
Add ShadeObject base class and Merchant model with camelCase (JSON) <-> snake_case (Python) mapping via a per-class alias table and a from_dict constructor. - ShadeObject: from_dict/to_dict alias mapping, ignores unknown keys, equality and repr driven by constructor params. - Merchant: explicitly typed fields (no generic settings dict); merchant_id coerced to int; address validated as a Stellar ed25519 public key (raises InvalidRequestError on construction); display_name computed property (business_name -> full name -> email). - Export Merchant and ShadeObject from the package. - Tests covering mapping, validation, display_name and round-tripping. Closes ShadeProtocol#39
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds ChangesMerchant model
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant API
participant ShadeObject
participant Merchant
participant StellarStrKey
API->>ShadeObject: from_dict(camelCase payload)
ShadeObject->>Merchant: construct filtered snake_case fields
Merchant->>StellarStrKey: validate address
StellarStrKey-->>Merchant: validation result
Merchant-->>ShadeObject: validated Merchant
ShadeObject-->>API: to_dict() camelCase payload
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/shade/merchant.py`:
- Line 39: In the public API field declaration for id, preserve the
schema-compatible name and add a targeted Ruff A002 suppression for this
intentional builtin shadowing. Do not rename the field or broaden the lint
suppression.
- Around line 57-58: Update the assignments in the merchant initializer to
accept only actual boolean values for active and verified, rejecting or
otherwise handling string and other non-boolean inputs instead of coercing them
with bool(). Add regression coverage confirming inputs such as "false" are not
silently converted to True.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 49965edc-1080-48fa-b836-8073ecad34dd
📒 Files selected for processing (4)
src/shade/__init__.pysrc/shade/base.pysrc/shade/merchant.pytests/test_merchant.py
|
GM @giftexceed |
Address maintainer review feedback on ShadeProtocol#45. - Replace bool() coercion of active/verified with a strict _require_bool check. bool("false") is True, so a malformed payload or a direct caller could silently flip a flag; non-bool values now raise InvalidRequestError with the offending param. - Add a targeted "noqa: A002" on the id parameter, keeping the schema-compatible public field name while satisfying the builtin-shadowing lint. - Add regression coverage asserting "false"/"true"/""/0/1/None are rejected for both flags, and that real booleans are preserved.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/shade/merchant.py (1)
76-80: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTrim
business_namebefore applying the fallback.A whitespace-only value such as
" "passes the truthiness check and is returned asdisplay_name, skipping the first/last-name and email fallbacks. Treat trimmed-empty business names as missing.Proposed fix
- if self.business_name: - return self.business_name + business_name = (self.business_name or "").strip() + if business_name: + return business_name🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shade/merchant.py` around lines 76 - 80, Update the business_name branch in the display_name logic to trim the value before checking or returning it, so whitespace-only names are treated as missing and the existing full_name and email fallbacks continue to run.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/shade/merchant.py`:
- Around line 76-80: Update the business_name branch in the display_name logic
to trim the value before checking or returning it, so whitespace-only names are
treated as missing and the existing full_name and email fallbacks continue to
run.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a7803b8e-0981-4650-8d4d-6b5d4cf29281
📒 Files selected for processing (2)
src/shade/merchant.pytests/test_merchant.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_merchant.py
Address maintainer review feedback on ShadeProtocol#45. A whitespace-only business_name such as " " passed the truthiness check and was returned verbatim, skipping the full-name and email fallbacks. Each candidate is now trimmed before it is tested and returned, so blank values fall through to the next one. The same latent bug applied to email, which was returned unnormalized as the final fallback; it is now trimmed too, and display_name yields None when every candidate is blank. The full-name branch already stripped. Add regression coverage for whitespace-only business_name, whitespace-only first/last names, an all-blank merchant, and trimming of the returned value.
|
Good catch — valid, and fixed in Reproduced it first: a Applied the suggested trim, and extended it to business_name = (self.business_name or "").strip()
if business_name:
return business_name
full_name = f"{self.first_name or ''} {self.last_name or ''}".strip()
if full_name:
return full_name
return (self.email or "").strip() or NoneOne resulting behavior change worth calling out: Regression coverage added for whitespace-only Validation: 163 tests passing (4 new), CI-critical flake8 ( |
codebestia
left a comment
codebestia
left a comment
There was a problem hiding this comment.
GM @giftexceed
Good job so far.
Here are some minor change to implement.
Update your branch with the main branch, then you will notice a new models folder.
Move your merchant.py model file into the model folder and use the already implemented ShadeObject base for your Merchant model.
Update the test accordingly as well.
Thank you.
Rebuild the Merchant model on the shared pydantic ShadeObject introduced on main (ShadeProtocol#47), replacing the standalone plain-Python model and base. - Move src/shade/merchant.py -> src/shade/models/merchant.py and delete the now-redundant src/shade/base.py. - Map camelCase JSON to snake_case fields with pydantic Field(alias=...); from_dict / to_dict / repr come from ShadeObject. - Enforce validation via pydantic: StrictBool for active/verified (no silent coercion of strings like "false"), a Stellar public-key field_validator on address, and a before-validator rejecting boolean merchant_id (which pydantic would otherwise coerce to 1/0). All surface as InvalidRequestError through the base. - Export Merchant from shade.models and the top-level package. - Update tests: unknown keys are now preserved (extra="allow"), the merchant_id error param is the "merchantId" alias, and add a boolean merchant_id rejection case.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/shade/models/merchant.py`:
- Around line 73-79: Update the full-name fallback in the merchant display-name
property or method to strip first_name and last_name individually, discard empty
components, and join the remaining values with a single space; preserve the
existing business_name and email fallback behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e3bf7297-2211-4856-96d5-289998fc622c
📒 Files selected for processing (6)
src/shade/__init__.pysrc/shade/base.pysrc/shade/merchant.pysrc/shade/models/__init__.pysrc/shade/models/merchant.pytests/test_merchant.py
|
@giftexceed |
Address maintainer review feedback on ShadeProtocol#45. The full-name fallback only stripped the combined string, so padded components leaked internal whitespace: first_name=" Ada " and last_name=" Lovelace " produced "Ada Lovelace". Strip each component and join the non-empty ones with a single space. Add regression coverage for padded name components.
codebestia
left a comment
codebestia
left a comment
There was a problem hiding this comment.
LGTM!
Nice implementation.
Thank you for your contribution.
2 participants
Summary
Implements the
Merchantmodel, mirroring the Shade backend's Prisma schema withcamelCase(JSON) →snake_case(Python) field mapping.merchant_id(from PrismamerchantId: Int) is the numeric identifier the Soroban contract stamps onto every invoice — the bridge between the backend and the on-chain world.Closes #39
Design note
The issue text suggested Pydantic, but Pydantic is not a dependency of this SDK (only
httpx+stellar-sdk) and the codebase is deliberately plain-Python. This PR implements the model in plain Python — matching the existing style, adding no new dependency, and reusingstellar-sdk(already present) for Stellar key validation. All acceptance criteria are met.Changes
ShadeObject(src/shade/base.py) — base class for API resources:from_dict/to_dictwith a per-class camelCase↔snake_case alias table, ignores unknown keys (additive backend changes stay safe), plus equality and repr.Merchant(src/shade/merchant.py):merchant_idcoerced toint(rejects bools/non-numerics).addressvalidated as a Stellar ed25519 public key (starts withG, 56 chars) → raisesInvalidRequestErroron construction.display_namecomputed property:business_name→"{first_name} {last_name}".strip()→email.MerchantandShadeObjectfrom the package.Acceptance criteria
Merchant.from_dict(api_response)maps camelCase → snake_case attributesmerchant.merchant_idis anintInvalidRequestErroron constructionmerchant.display_namereturns the most informative available nameTesting
tests/test_merchant.py— 14 tests covering mapping, unknown-key tolerance,merchant_idtyping, address validation (bad key + wrong length),display_namefallbacks, optional defaults, and camelCase round-tripping.Summary by CodeRabbit
Merchantmodel with validated fields anddisplay_nameresolution/precedence.to_dict/from_dictround-trip serialization and stable equality.merchant_id, Stellar ed25519address, and boolean flags (active/verified), with request errors indicating the failing parameter.display_nametrimming/precedence, export behavior, and serialization round-trips.